Skip to content

Add a feature to pass user provided files during qcow2ova image conversion - #851

Merged
ppc64le-cloud-bot merged 1 commit into
ppc64le-cloud:mainfrom
Amulyam24:qcow2ova-add-files
Aug 27, 2026
Merged

Add a feature to pass user provided files during qcow2ova image conversion#851
ppc64le-cloud-bot merged 1 commit into
ppc64le-cloud:mainfrom
Amulyam24:qcow2ova-add-files

Conversation

@Amulyam24

@Amulyam24 Amulyam24 commented Jan 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
This PR adds a feature to allow user to add custom files to the image built.
--write-files-list is a comma separated list of files and --write-to-dir-path is the path where the files are written in the built image

example:

pvsadm image qcow2ova --image-name test-image --image-dist centos --image-url https://cloud.centos.org/centos/10-stream/ppc64le/images/CentOS-Stream-GenericCloud-10-latest.ppc64le.qcow2 --prep-template ./image-prep.template --write-files-list key.pub,files --write-to-dir-path /home/user

Tested the following scenarios

  1. Copy multiple files and directories
  2. Copy nested directories
  3. If destination path doesn't exist, creates and copies to it

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #811

Special notes for your reviewer:

**Output/Demonstration

Add a feature to pass user provided files during qcow2ova image conversion

@ppc64le-cloud-bot ppc64le-cloud-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 8, 2026
@Prajyot-Parab

Prajyot-Parab commented Jun 10, 2026

Copy link
Copy Markdown
Member

/cc @kishen-v

@kishen-v

kishen-v commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@ppc64le-cloud-bot ppc64le-cloud-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 1, 2026
kishen-v
kishen-v reviewed Jul 1, 2026
View reviewed changes
Comment thread cmd/image/qcow2ova/prep/prepare.go Outdated
@Amulyam24
Amulyam24 force-pushed the qcow2ova-add-files branch from 0d7a269 to d21a092 Compare August 3, 2026 07:04
@ppc64le-cloud-bot ppc64le-cloud-bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2026
@Amulyam24
Amulyam24 requested a review from kishen-v August 3, 2026 07:05
@Amulyam24

Amulyam24 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

@kishen-v @anup-kodlekere, PTAL!

@anup-kodlekere

anup-kodlekere commented Aug 3, 2026

Copy link
Copy Markdown

LGTM!

@kishen-v

kishen-v commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@ppc64le-cloud-bot ppc64le-cloud-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2026
mkumatag
mkumatag reviewed Aug 4, 2026
View reviewed changes

@mkumatag mkumatag left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Thanks for the feature! I've left inline comments on the specific issues below. Summary of what needs attention before merge:

  • 🔴 [High] Missing flag co-validation — --write-files-list without --write-to-dir-path silently dumps files into the image root
  • 🟡 [Medium] coreos silently drops user-provided files with no warning
  • 🟡 [Medium] MkdirAll runs unconditionally even when no files are specified
  • 🟠 [Medium] No path traversal guard on --write-to-dir-path
  • 🟡 [Low] os.Stat in copyFiles follows symlinks transparently
  • 🟢 [Low] Double stat call in copyFile (minor)
  • 🟡 [Medium] Broken example command (missing --image-name value)
  • 🟡 [Medium] No unit tests for the new copy logic

Also worth considering: the flag names --write-files-list and --write-to-dir-path are more verbose than the project style. Something like --inject-files + --inject-dest would be terser and more intent-driven, matching how --prep-template, --cloud-config, --temp-dir are named.

Review done by Bob

Comment thread cmd/image/qcow2ova/prep/prepare.go
Comment thread cmd/image/qcow2ova/prep/prepare.go
Comment thread cmd/image/qcow2ova/prep/prepare.go
Comment thread cmd/image/qcow2ova/prep/prepare.go
Comment thread cmd/image/qcow2ova/prep/prepare.go
Comment thread cmd/image/qcow2ova/prep/prepare.go
mkumatag
mkumatag reviewed Aug 4, 2026
View reviewed changes
Comment thread cmd/image/qcow2ova/qcow2ova.go
@ppc64le-cloud-bot ppc64le-cloud-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 21, 2026
@Amulyam24

Amulyam24 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @mkumatag, I've addressed the comments and added relevant UT, PTAL!

@Amulyam24
Amulyam24 requested a review from mkumatag August 21, 2026 09:42
@Amulyam24

Amulyam24 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Tested the latest changes

pvsadm image qcow2ova --image-name test-image --image-dist centos --image-url https://cloud.centos.org/centos/10-stream/ppc64le/images/CentOS-Stream-GenericCloud-10-latest.ppc64le.qcow2 --prep-template ./image-prep.template --write-files-list a.txt,keys --write-to-dir-path /home/user

Output from the test VM created using this image

[root@test ~]# ls /home/user/
a.txt  keys
[root@test ~]# cat /home/user/a.txt
hello
[root@test ~]# ls /home/user/keys
key.priv  key.pub
[root@test ~]# cat /home/user/keys/key.priv
private key
[root@test ~]# cat /home/user/keys/key.pub
public key

mkumatag
mkumatag approved these changes Aug 27, 2026
View reviewed changes

@mkumatag mkumatag left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@ppc64le-cloud-bot ppc64le-cloud-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2026
@ppc64le-cloud-bot

ppc64le-cloud-bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Amulyam24, mkumatag

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ppc64le-cloud-bot ppc64le-cloud-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2026
@ppc64le-cloud-bot
ppc64le-cloud-bot merged commit 4d84a53 into ppc64le-cloud:main Aug 27, 2026
5 checks passed
@Amulyam24
Amulyam24 deleted the qcow2ova-add-files branch August 28, 2026 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mkumatag mkumatag mkumatag approved these changes
@sudeeshjohn sudeeshjohn Awaiting requested review from sudeeshjohn
@kishen-v kishen-v Awaiting requested review from kishen-v

Assignees

@mkumatag mkumatag

@kishen-v kishen-v

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add feature to pass or mount user files during image qcow2ova conversion

6 participants

@Amulyam24 @Prajyot-Parab @kishen-v @anup-kodlekere @ppc64le-cloud-bot @mkumatag